Improved Non-Equi Join Explanation in data.table #6813
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6813 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 79 79
Lines 14650 14650
=======================================
Hits 14452 14452
Misses 198 198 ☔ View full report in Codecov by Sentry. |
|
Yes, this adds what I thought was missing from the tutorial. I think it can be communicated a bit more concisely. (I'll make a concrete suggestion. Also, you may consider having your example use the |
katrinabrock
left a comment
katrinabrock
left a comment
There was a problem hiding this comment.
hah it seems @tdhock and I were making similar comments at the same time. These are just my suggestions on how to make it more clear an concise. I'm not a maintainer so you don't have to listen to me. :-D
|
i made the changes as suggested. |
venom1204
force-pushed
the
issue6626
branch
2 times, most recently
from
3f484c3 to
63369a1
Compare
|
"I've updated the section by removing backticks from non-code parts as suggested. However, I've retained them where necessary, such as around data.table, column names like A in A >= B, and function names to maintain clarity and consistency." @tdhock can you please review ths and suggest any changes if required |
Co-authored-by: Brock <[email protected]>
katrinabrock
left a comment
katrinabrock
left a comment
There was a problem hiding this comment.
Looks good to me. I didn't compile to make sure the rmd syntax is working.
Also, I still think it's a good idea to use a more realisitic/less abstract dataset for the examples, but fine as is.
Again, though, I'm not a maintainer so my approval is not so important.
Co-authored-by: Michael Chirico <[email protected]>
Co-authored-by: Michael Chirico <[email protected]>
MichaelChirico
left a comment
MichaelChirico
left a comment
There was a problem hiding this comment.
Thanks! One last round of revisions should do it I think.
Co-authored-by: Michael Chirico <[email protected]>
Co-authored-by: Michael Chirico <[email protected]>
@MichaelChirico Done! Kindly have a look when you have time. |
MichaelChirico
left a comment
MichaelChirico
left a comment
There was a problem hiding this comment.
Looking good, thank you!
|
Be sure to check the comparison from the initial PR commit till now as material for review & future improvement :) 43b8bcb...c0f86bf#diff-4842b9a4870c362deab64182bd9e511b2f057db49e7169b21a37024a09ef4700 |
4 participants
closes #6623
in this I
This update enhances clarity and ensures better understanding of non-equi join behavior in data.table.
hi @tdhock @katrinabrock @MichaelChirico can you please review and suggest me if further improvements are required or not.
thank you .